Skip to content

Eliminate use of StopRun(true) - #1858

Open
CharliePoole wants to merge 3 commits into
mainfrom
issue-1856
Open

Eliminate use of StopRun(true)#1858
CharliePoole wants to merge 3 commits into
mainfrom
issue-1856

Conversation

@CharliePoole

@CharliePoole CharliePoole commented Jul 28, 2026

Copy link
Copy Markdown
Member

Eliminates use of StopRun(bool) in the engine and agent core and fixes #1856.

StopRun(false) is replaced by RequestStop(), which is intended to make it clear that the test may not stop. The caller may only request it and the result depends on the test framework, possibly its version and the test code itself.

StopRun(true) is replace by ForcedStop(). If the test framework provides any facilities for such a stop, they are used. Otherwise, any non terminated processes are canceled by the engine.

The initial commit makes the necessary replacements but code to kill agent processes is not yet implemented.

@CharliePoole
CharliePoole marked this pull request as draft July 28, 2026 20:20
@CharliePoole

CharliePoole commented Jul 29, 2026

Copy link
Copy Markdown
Member Author

@manfred-brands
I'm keeping this as a draft until a few things are resolved...

  1. I'd like to see how you resolve Eliminate forced stop from the API nunit#5373 and accommodate the engine fix to whatever you do in the driver code.
  2. Deciding whether to Kill test processes from the engine in response to a forced stop and, if so, where to do it.
  3. Deciding whether to continue to fabricate TestSuite results in the engine after a forced stop or to expect clients (runners) to deal with it. In the PR, that code is currently removed using conditional compilation, so we can test the adapter with it.

It would be be useful if you'd take a look at the PR as it is now and let me know what you think.

@manfred-brands

manfred-brands commented Jul 29, 2026

Copy link
Copy Markdown
Member
  1. I'd like to see how you resolve Eliminate forced stop from the API nunit#5373 and accommodate the engine fix to whatever you do in the driver code.

I have raised nunit/nunit#5374

I left the signature for the FrameworkController the same (similar what you did earlier).
That way existing runners should still work (unless they set force to true)

@CharliePoole

CharliePoole commented Jul 29, 2026

Copy link
Copy Markdown
Member Author

On the other items...

  1. Deciding whether to Kill test processes from the engine in response to a forced stop and, if so, where to do it.

I'll have the agent just terminate itself when it receives a ForcedStop() call.

  1. Deciding whether to continue to fabricate TestSuite results in the engine after a forced stop or to expect clients (runners) to deal with it. In the PR, that code is currently removed using conditional compilation, so we can test the adapter with it.

For now, I'm going to turn the tracker code back on.

@CharliePoole
CharliePoole marked this pull request as ready for review July 30, 2026 14:48
@CharliePoole

CharliePoole commented Jul 30, 2026

Copy link
Copy Markdown
Member Author

@manfred-brands This is ready for review. I've left a few things for a later issue/PR...

  1. Somebody up the hierarchy should probably wait to see if the job was cancelled. This is hard to test under the console runner. My GUI has menu options for Stop and Kill, so I'll test it there once it's using the latest NUnit code. I'm thinking that the best place for this is probably in each runner, but we'll see.
  2. Right now, I'm guessing a bit about what will be in the final 5.0 release of the framework, so changes may be needed when that comes out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@manfred-brands manfred-brands Awaiting requested review from manfred-brands

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Eliminate forced stop from the API

2 participants

@CharliePoole @manfred-brands